-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feat/style: landing page #7
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Hmmmm, I'll look into it |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The landing page page.ts
downloaded to the browser is about 18mb
this takes a while to load and longer till interactivity.
This may be due to this bottleneck #7 (review)
A fix will be included in the next PR, for now lets get this merge as 2 other separate PR's depends on this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Post-Merge Review
Great work, @IgboPharaoh! I’ve left a few inline comments on the code, but nothing major.
Below are the issues that Tobi and I found during our review, comparing the current UI against the designs:
Header Section
- Logo:
- Desktop: The logo's width and height are incorrect; it should be larger according to the designs. This also affects the padding, which looks off.
- Mobile: The logo's width is incorrect; it should be larger based on the designs.
Hero Section
- Background Orange Circles: The circles are missing. You can export them from Figma the same way you exported the transcript cards for the hero section.
- Body Text: There is an awkward sentence break in the mobile (and desktop) view. Ideally, the text should fit on one line, and if it doesn’t, the second sentence should appear on its own line.
Featured Transcripts Section
- Background Color: The background color doesn’t match the designs. Notice how it transitions when moving between sections.
- Card Shadows: The card shadows need to be softer, as seen in the designs on Figma. The shadows for the cards in the "Explore Transcripts" section seem fine, so it’s just an issue here.
- "x days ago": The text color is incorrect compared to the designs.
Why Transcripts Section
- Background Color: Similar to the "Featured Transcripts" section, the background color is incorrect. Refer to the designs for how it should transition between sections.
- First Sub-Section:
- The copy doesn’t match the Figma file; please update it to reflect the correct text.
- Replace the placeholder image with the GIF from the designs.
- Sub-Section Heading: The padding is too large; it should be 18px instead of 24px. The font also needs to be slightly bolder to match the design specs.
- Sub-Section Body Text: The text stretches too much on larger screens. Refer to the Bitcoin Search landing page for a similar layout; we should aim for a consistent approach.
"README.md", | ||
"STYLE.md", | ||
"twitter_handles.json", | ||
".json", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, I forgot the contentDirExclude
when I did the refactoring.
Does ".json"
means that it's excluding all the JSON files?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No it doesn't, this and other things in the contentDirExclude
array just excludes files at the root directory
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So it excludes all JSON files in the root directory? If that's the case then you don't need to separate specify "twitter_handles.json"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let me tag this comment to the pr in prod
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please check the designs, we are using a different word-cloud image.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You added this circle-background but I can't see it on the actual page.
Also, there is another outside circle as part of the hero design.
import Header from "./Header"; | ||
import HeroSection from "./HeroSection"; | ||
import Logo from "./Logo"; | ||
import Wrapper from "./Wrapper"; | ||
import FeaturedTranscripts from "./FeaturedTranscripts"; | ||
import TranscriptCard, { ExploreTranscriptCard } from "./TranscriptCard"; | ||
import ExploreTranscripts from "./ExploreTranscripts"; | ||
import FooterTop from "./FooterTop"; | ||
import WhyTranscripts from "./WhyTranscripts"; | ||
import FooterComponent from "./FooterComponent"; | ||
import SuggestModal from "./SuggestionModal"; | ||
import ExploreTranscriptClient from "./ExploreTranscriptClient"; | ||
import FeaturedTranscriptClient from "./FeaturedTranscriptClient"; | ||
|
||
export { | ||
Header, | ||
HeroSection, | ||
Logo, | ||
Wrapper, | ||
FeaturedTranscripts, | ||
FeaturedTranscriptClient, | ||
TranscriptCard, | ||
ExploreTranscripts, | ||
FooterTop, | ||
WhyTranscripts, | ||
FooterComponent, | ||
SuggestModal, | ||
ExploreTranscriptClient, | ||
ExploreTranscriptCard, | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like using index.ts
files as they allow us to export everything from a central location. However, we've been following a different export pattern across the rest of the projects, and I believe it would be best to maintain consistency.
For reference, here's the pattern used in another file (src):
export * from "./review.routes";
export * from "./transaction.routes";
export * from "./transcript.routes";
export * from "./user.routes";
And the alternative we use elsewhere (src):
export { default as ReviewGuidelinesModal } from "./ReviewGuidelinesModal";
export { default as SubmitTranscriptModal } from "./SubmitTranscriptModal";
export { default as RestoreOriginalModal } from "./RestoreOriginalModal";
export { default as SuggestModal } from "./SuggestModal";
Personally, I prefer the second approach, but I’m open to discussing this further to see what works best for everyone.
cc @Emmanuel-Develops
{ | ||
href: "https://bitcoindevs.xyz/", | ||
image: bitcoindevs, | ||
alt: "Bitcoin Devs", | ||
title: "Build the future of money - Study & contribute to bitcoin and lightning open source", | ||
}, | ||
{ | ||
href: "https://chat.bitcoinsearch.xyz", | ||
image: chatbtc, | ||
alt: "ChatBTC image", | ||
title: "Interactive AI chat to learn about bitcoin technology and its history", | ||
}, | ||
{ | ||
href: "https://bitcoinsearch.xyz/", | ||
image: bitcoinSearch, | ||
alt: "Bitcoin search", | ||
title: "Technical bitcoin search engine", | ||
}, | ||
{ | ||
href: "https://tldr.bitcoinsearch.xyz/", | ||
image: bitcointldr, | ||
alt: "Bitcoin TLDR", | ||
title: "Daily summary of key bitcoin tech development discussions and updates", | ||
}, | ||
{ | ||
href: "https://savingsatoshi.com", | ||
image: savingSatoshi, | ||
alt: "Saving Satoshi", | ||
title: "Engaging bitcoin dev intro for coders using technical texts and code challenges", | ||
}, | ||
{ | ||
href: "https://review.btctranscripts.com/", | ||
image: transcriptsreview, | ||
alt: "Bitcoin Transcripts Review", | ||
title: "Review technical bitcoin transcripts and earn sats", | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Emmanuel-Develops, does it make sense to store this information, along with the logos for each product, in the components library as a central source of truth? We're currently repeating them across various products, and in some cases, the copy is even inconsistent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes they should be in the component library. Given that the library should be lightweight it would not be wise to have the images in there, rather they should point to where they are hosted. Ideally somewhere on cloudflare.
Cloudflare has a limited free image hosting and that would serve our purposes for now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was mostly thinking about the text data, but your point regarding the images is spot on! I just opened an issue in order to keep track of this bitcoin-dev-project/bdp-ui#30
* build: install bdp-ui icons * build: set config requirements * style: add assets and icons * chore: remove redundant styles, add menu data * chore: add landing page components * chore: create landing page with responsive styles * fix: build error * replace image assets * add mobile menu and app menu * install bdp-ui from npm * complete landing page requirements * remove search icon, change image path and add priority prop
* simplify contentlayer.config (#6) a simplification of the config alongside some changes to the pages in order to show the structure based on the new simplified implementation. * Feat/style: landing page (#7) * build: install bdp-ui icons * build: set config requirements * style: add assets and icons * chore: remove redundant styles, add menu data * chore: add landing page components * chore: create landing page with responsive styles * fix: build error * replace image assets * add mobile menu and app menu * install bdp-ui from npm * complete landing page requirements * remove search icon, change image path and add priority prop * feat: Add hugo built pages (#15) * feat: fetch and render hugo built pages in nextJs * refactor: use rewrites in place of [...slug] * Fix/landing page build (#18) * add required assets * enhance contentlayer algorithm * add design corrections * filter languages that are not english * remove unnecessary console.log * capitalize titles and sort types * refactor code structure * add product favicon * chore: rm commented code * fix: hash navigation, change stock image to btc prague conference image (#24) --------- Co-authored-by: Andreas <[email protected]> Co-authored-by: Solomon eze <[email protected]>
LANDING PAGE